-
Notifications
You must be signed in to change notification settings - Fork 267
Add azd auth status command
#6377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
azd auth status command
wbreza
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the idea - just have some open questions about the flags.
@copilot - Please review token caching and if we support multiple tokens with different tenants/scopes.
Reviewed the auth system in commit bb9541f. The system stores a single "current user" credential (not multiple tokens). When Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a new azd auth status command to provide a more intuitive way to check authentication status, replacing the previously required azd auth login --check-status flag. The implementation maintains backward compatibility while improving user experience by following CLI conventions used by tools like the GitHub CLI.
Key Changes
- New dedicated
azd auth statuscommand with support for both text and JSON output formats - Enhanced credential expiration display in both
azd auth statusandazd auth login --check-status - Updated command autocomplete and help documentation
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
cli/azd/cmd/auth_status.go |
New command implementation with authentication status checking, credential verification, and output formatting |
cli/azd/cmd/auth.go |
Registers the new status subcommand under the auth command group with JSON and text output format support |
cli/azd/cmd/auth_login.go |
Updates --check-status flag behavior to display credential expiration time for consistency |
cli/azd/cmd/testdata/TestUsage-azd-auth.snap |
Adds status subcommand to auth command help output |
cli/azd/cmd/testdata/TestUsage-azd-auth-status.snap |
New snapshot test for auth status command help text |
cli/azd/cmd/testdata/TestFigSpec.ts |
Updates Fig autocomplete specification to include the new status subcommand |
|
@copilot apply changes based on the comments in this thread |
JeffreyCA
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/azp run azure-dev - cli
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@weikanglim could you clarify what you're suggesting should be changed? @copilot could you fix the golangci-lint warnings? |
Fixed golangci-lint warnings in commit 8bb30cb. Split long function signatures and type definitions in |
|
@copilot There are still some lingering failures, can you check by running |
|
I agree with @weikanglim. Users may be confused about the access token expiration time and mistakenly believe that their login session will expire soon. We should either remove the expiry display (and add ID token credential expiration to the logs if necessary) or add clarifying text such as "Access token credential expires on: ...". Depending on telemetry data, if we find that most users rely on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to take another pass on this.
High level issues found:
- The access token expiration date printed out is meaningless for users.
- The chosen implementation of tying auth status to
LoginResultused forloginis going to make future changes harder to implmement, such as #5404 - The command title deserves something more specific over "Check the authentication status"
To save everyone's time and effort, I'm going to push the changes privately and iterate with a local coding agent rather than continuing the trend of iterating with copilot via under-specified specifications over PR comments.
cc @JeffreyCA who was wondering about the blocking status here
1d753eb to
0fd3728
Compare
|
/azp run azure-dev - cli |
|
Azure Pipelines successfully started running 1 pipeline(s). |
0fd3728 to
5ba25ee
Compare
weikanglim
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restting approval after pushing changes
|
/azp run azure-dev - cli |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
Summary
Adds new
azd auth statuscommand to display current authentication state and account information.Changes
azd auth statuscommand — shows whether user is logged in and account detailsStatusResultcontract withstatus,type,email, andclientIdfields. This is wrapped in anAuthStatusViewfor unformatted/ux otuput.AuthStatustype (authenticated/unauthenticated) — separate fromLoginStatusto avoid breaking existingazd auth loginoutputAccountTypetype (user/servicePrincipal)Preview changes
User:
{ "status": "authenticated", "type": "user", "email": "<email>" }Principal:
{ "status": "authenticated", "type": "servicePrincipal", "clientId": "<guid>" }Architecture Notes
Introduces a pattern for command output:
contracts.*— pure data schemas for--output jsonux.*View— wraps contracts, implementsToString()/MarshalJSON()for console displayTesting
Added functional tests in
test/functional/auth_test.go:Test_CLI_AuthStatus_Authenticated— uses external auth mockTest_CLI_AuthStatus_Unauthenticated/_Text— uses empty config dirFixes #5404 #2898